[release-1.13] 🐛 fix: use machine's .spec.failureDomain with fallback to vspheremachine's .spec.failureDomain if not defined #3588
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @jcpowermac. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
the
size/XS
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
/ok-to-test |
controllers/vspherevm_controller.go
Outdated
| if failureDomain != "" { | ||
| vsphereDeploymentZone := &infrav1.VSphereDeploymentZone{} | ||
| if err := r.Client.Get(ctx, apitypes.NamespacedName{Name: *failureDomain}, vsphereDeploymentZone); err != nil { | ||
| return reconcile.Result{}, errors.Wrapf(err, "failed to get VSphereDeploymentZone %s", *failureDomain) |
There was a problem hiding this comment.
| return reconcile.Result{}, errors.Wrapf(err, "failed to get VSphereDeploymentZone %s", *failureDomain) | |
| return reconcile.Result{}, errors.Wrapf(err, "failed to get VSphereDeploymentZone %s", failureDomain) |
controllers/vspherevm_controller.go
Outdated
| return ctrl.Result{}, err | ||
| } | ||
|
|
||
| failureDomain := machine.Spec.FailureDomain |
There was a problem hiding this comment.
| failureDomain := machine.Spec.FailureDomain | |
| failureDomain := ptr.Deref(machine.Spec.FailureDomain, "") |
… to vspheremachine's .spec.failureDomain if not defined This commit sets failureDomain string defaulting to `machine.Spec.FailureDomain`. If failureDomain is an empty string then `vsphereMachine.Spec.FailureDomain` is checked for nil, if defined `failureDomain` is set to `vsphereMachine.Spec.FailureDomain`. This resolves an issue where virtual machines are not placed within vm-host groups in a timely manner causing misplacement in the topology.
jcpowermac
force-pushed
the
cp-to-113-fix-fd-race
branch
from
4ba6045 to
c183af6
Compare
k8s-ci-robot
added
size/S
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: abf773c73052800c5afa295673d3f65e675cabc9
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chrischdi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/cherry-pick release-1.12 |
|
@chrischdi: once the present PR merges, I will cherry-pick it on top of In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@chrischdi: new pull request created: #3590 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
4 participants
Manual cherry-pick of #3576
This commit sets failureDomain string defaulting to
machine.Spec.FailureDomain. If failureDomainis an empty string then
vsphereMachine.Spec.FailureDomainis checked for nil, if definedfailureDomainis set tovsphereMachine.Spec.FailureDomain.This resolves an issue where virtual machines are not placed within vm-host groups in a timely
manner causing misplacement in the topology.